Skip to content

feat(sql-editor): add compact worksheet filters and ranges - #111027

Open
mariusandra wants to merge 6 commits into
masterfrom
feat/bi-quick-filters
Open

mariusandra wants to merge 6 commits into
masterfrom
feat/bi-quick-filters

Conversation

@mariusandra

@mariusandra mariusandra commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

BI worksheets need category selection and ranges without sacrificing space for fields and results. Builds on calculated measures in #110995.

Changes

  • Compact filter pills sit beside or below the data pane, with quick value controls on the right of the results.
  • Quick filters use two columns on wide worksheets. Tight scenes retain editing through the left-side pills; detailed controls open in popovers.
  • Is any of and Is none of support multiple categories, searchable suggestions, and typed values. Clear all restores unrestricted values.
  • Suggestions respect other applied filters, refresh while focused, and return up to 100 distinct values from the selected source.
  • Between supports inclusive numeric and date-time bounds, including open-ended ranges.
  • Numeric values retain their precision. Invalid numbers show an inline error and block automatic and manual query runs.
  • Apply filter temporarily disables a filter while preserving its settings. Changes respect auto-update and persist in the worksheet configuration.

Current layout: eight filters at 1600px and 1050px, using synthetic Storybook data.

Compact filters, wide

Compact filters, narrow

Earlier screenshots, superseded by the compact layout above

Original single-value filter editor:

Before

Earlier category selection layout:

Category picker

Date range and narrow layout

Date range
Narrow layout

Earlier numeric validation layout:

Invalid numeric filter

How did you test this code?

Local checks: scoped query/editor Jest suites, frontend lint/format, typecheck, and CI preflight.

Playwright exercised suggestions, multi-selection, clear-all, disable/re-enable, date changes, numeric ranges including zero, and manual Run with auto-update disabled. Screenshots show the rendered controls at 1600px and 1050px.

Test rationale: Extended the existing query-generation tests for escaping, range bounds, suggestion scoping, and persisted-state validation. Extended the URL-restoration test for filter toggles and operator conversion.
A focused-input regression test verifies suggestions reload after the query changes. Chromium also verified refresh without losing input focus.
Regression cases cover large integers, high-precision decimals, invalid numeric values, and preventing stale-query execution after validation fails.
Chromium verified exact large-integer bounds, invalid range/list errors, blocked automatic/manual runs, recovery after correction, and wide/narrow error layouts.
The compact layout was checked with eight filters, editing from both sides, toggles, operator changes, numeric validation, and a 520px worksheet container.

Live ClickHouse and external warehouse execution were not tested; browser query responses use synthetic fixtures.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

Updated the existing data warehouse engineering guide with filter behavior.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Codex, GPT-6

Skills: debugging-ci-failures, stacking-prs, writing-ui-components, writing-kea-logics, placing-product-frontend-code, writing-user-facing-copy, writing-tests, writing-clickhouse-queries, adopting-generated-api-types, setting-feature-flags-in-storybook, running-ci-preflight, reviewing-with-coderabbit, writing-pr-descriptions.

The layout reference informed control types only. Committed fixtures and uploaded screenshots contain invented sample data, with no reference image content.

Local CodeRabbit review skipped: CLI signed out.

@mariusandra mariusandra self-assigned this Oct 2, 2026
@mariusandra
mariusandra added this pull request to stack #110997 October 2, 2026 17:12
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Complexity (TypeScript) — 22 functions above the limit (max 80)

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

Function Location Complexity Limit
<anonymous> frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3730 80 10
createQueryTab frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3881 72 10
parseBIEditorState frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:426 38 10
getBIFilterSummary frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:674 22 10
filterExpression frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:719 22 10
createTab frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2052 21 10
runQuery frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2263 21 10
saveAsViewSubmit frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2560 20 10
getBIChartFit frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:322 18 10
updateInsight frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3038 18 10
discardChanges frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3125 18 10
runAfterChange frontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.ts:879 17 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:4215 16 10
<anonymous> products/data_warehouse/frontend/bi/BIFilterValueInput.tsx:72 16 10
parseBIFieldValue frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:394 15 10
<anonymous> frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:485 15 10
buildBIQuery frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:986 15 10
persistEditorDraft frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:1815 15 10
BIFilterValueInput products/data_warehouse/frontend/bi/BIFilterValueInput.tsx:17 14 10
renderQueryOutline frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:166 13 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3553 13 10
addFieldToConfig frontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.ts:96 11 10
✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Bundle size — 🔺 +7.5 KiB (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 69.87 MiB · 🔺 +7.5 KiB (+0.0%)

File Size Δ vs base
exporter/src/exporter/scenes/ExporterNotebookScene.js 3.91 MiB 🔺 +7.5 KiB (+0.2%)

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.64 MiB · 23 files 🔺 +690 B (+0.0%) █████████░ 89.3% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.73 MiB · 662 files 🔺 +690 B (+0.0%) █████████░ 92.6% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.60 MiB · 2,411 files 🔺 +690 B (+0.0%) █████████░ 91.1% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.69 MiB · 3,401 files 🔺 +690 B (+0.0%) ███████░░░ 71.9% of 13.48 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.61 MiB · 2,419 files 🔺 +690 B (+0.0%) █████████░ 88.7% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.31 MiB · 3,253 files 🔺 +690 B (+0.0%) ███████░░░ 73.7% of 12.64 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.14 MiB · 4,139 files 🔺 +690 B (+0.0%) ████████░░ 77.3% of 15.72 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
839 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.5 KiB src/lib/api.ts
92.7 KiB src/products.tsx
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
40.1 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
29.0 KiB ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
111.8 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
111.8 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
111.8 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
111.8 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
315.5 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
111.8 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.21 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.21 MiB · 19 files 🔺 +690 B (+0.0%) ████░░░░░░ 38.6% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
844.1 KiB dist/toolbar/toolbar-app-HUNK4GB6.css
657.6 KiB dist/toolbar/chunk-chunk-FH4DAUHX.js
259.4 KiB dist/toolbar/chunk-chunk-7JWMBALG.js
138.2 KiB dist/toolbar/chunk-chunk-7OHQ36LB.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-HB2CVGL6.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-L2WFOQQE.js
21.0 KiB dist/toolbar/chunk-chunk-IO6BQT46.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +121.5 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 961.49 MiB · 🔺 +121.5 KiB (+0.0%)

✅ Playwright — all passed

All tests passed.

View test results →

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Adds filter operators and UI for worksheet data filtering.

The PR appears safe to merge, with a non-blocking calculated-measure column-naming issue.

Reviews (2) · Last reviewed commit: "fix(sql-editor): validate numeric worksh..."

Comment thread frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts Outdated
Comment thread frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts
@mariusandra
mariusandra force-pushed the feat/bi-quick-filters branch from 84686da to 9ee12a5 Compare October 2, 2026 17:26
@trunk-io

trunk-io Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes-App/Replay Vision/Observations tab in the player TimelineWithScans smoke-test The test failed because the player controls were still visible when they were expected to be hidden. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 522d3a44-c6dd-4830-a610-0ab15ba6bc8b
📥 Commits

Reviewing files that changed from the base of the PR and between 2ba4199 and a8335de.

📒 Files selected for processing (3)
  • frontend/snapshots.yml
  • frontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsx
  • frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

BI worksheet filters now support multi-value inclusion and exclusion, inclusive ranges, and a disabled state that retains filter settings. The editor renders controls for these filters and loads distinct-value suggestions on demand. Invalid filters block worksheet queries. Filter changes integrate with worksheet state and URL persistence. Tests, Storybook stories, layout updates, and handbook documentation cover the changes.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to a8335

The narrow worksheet story can render, and invalid numeric ranges cannot run as BI queries. No actionable merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a8335

The changes remain focused on worksheet filtering and querying. Numeric validation adds an execution safeguard, and no new security issue was demonstrated. Tenant authorization and every cancellation or recovery state were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The new production access path is suggestion querying for the worksheet-selected source and connection. The 100-value limit bounds returned suggestions, not authorization or database scan cost. Effective tenant and connection permissions remain dependent on the existing query service, whose enforcement was not inspected.

Trust Boundaries and Controls

  • inferred — Worksheet filters are user-controlled query predicates rather than an authorization boundary: users can disable them, clear selections, use custom expressions, or explicitly execute SQL overrides. Numeric validation protects generated filter syntax; it does not establish tenant or asset authorization.

Resilience and Maintainability Implications

  • observed — Suggestion state is keyed by the serialized query, and the loader checks its breakpoint after execution before publishing values. Failures expose an error and retry action while retaining typed-value entry. Suggestion responses supply options rather than directly mutating applied filter values; complete server-side cancellation was not verified.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description explains the user problem, visible changes, testing, release status, and documentation update. It includes screenshots and agent context. The agent context does not include a session l…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (2)
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts-669-672 (1)

669-672: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Range bounds on numeric fields send any string that is not a number as a string literal.

literal falls back to escapeHogQLString for numeric fields when the input is not a finite number. For example, a user can type abc into a revenue range. The generated SQL is then properties.revenue >= 'abc'. ClickHouse either rejects the query or compares it as a string, so the range silently returns wrong rows. The scalar path behaves the same way, but between uses a free-text LemonInput with no number validation. Reject the invalid bound, or skip it and show the input as invalid.

products/data_warehouse/frontend/bi/BIFilterValueInput.tsx-16-18 (1)

16-18: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reload suggestions when the query changes while the selector is open.

BIFilterValueInput calls loadOptions() only from LemonInputSelect.onFocus. A changed query creates a logic state with options === null, but LemonInputSelect keeps the popover open and does not fire onFocus when options changes. The menu can therefore show empty or stale suggestions until the user focuses the selector again.

Trigger loading when the popover opens or when its query changes. The query excludes the current filter, so remove the claim that selecting this filter's own values changes the key. The inspected code also does not establish that old Kea instances remain mounted.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 8a548893-2384-4e2b-ba87-bdf578ca8caa

📥 Commits

Reviewing files that changed from the base of the PR and between 38ec84d and 9ee12a5.

📒 Files selected for processing (14)
  • docs/published/handbook/engineering/data-warehouse.md
  • frontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.ts
  • frontend/src/scenes/data-warehouse/editor/bi/biEditorOptions.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.test.ts
  • frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFilterEditor.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFilterPill.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFiltersCard.tsx
  • frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.test.ts
  • products/data_warehouse/frontend/bi/BIFilterControl.tsx
  • products/data_warehouse/frontend/bi/BIFilterValueInput.tsx
  • products/data_warehouse/frontend/bi/biFilterValuesLogic.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.

@mariusandra
mariusandra force-pushed the feat/bi-quick-filters branch from 9ee12a5 to 85d889a Compare October 2, 2026 18:33
@mariusandra
mariusandra marked this pull request as ready for review October 2, 2026 18:45
@parameterai

parameterai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Risk: No findings

This delta tightens BI editor state handling: data-pane loading/error/hydration selectors now require the worksheet's source connection to match the active database connection, and filter suggestions load through an explicit active (focus) prop that reloads when the query changes. No new security issues; the gating changes only affect display state and field hydration, and the suggestion query path re-uses the previously reviewed, validated HogQL construction.

Sentinel reviewed d1cf9d1 · Review settings

@greptile-apps

greptile-apps Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings could not be posted inline.

  • P2 Measure alias changes unnecessarily frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:798 ▶

    A calculated measure labeled sum with the formula sum(revenue) gets the output column name sum_2: this check treats the function name inside the formula as an alias collision, even when no field or column is named sum. That makes the worksheet’s output column differ from the chosen label. Check for identifier collisions rather than substrings so the column keeps its name.

@github-actions
github-actions Bot requested a deployment to preview-pr-111027 October 2, 2026 19:01 In progress
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit c32f71c · box box-57f7b223da39 · ready in 611s (push → usable) · build log · rebuilds on every push, torn down on close

@mariusandra mariusandra changed the title feat(sql-editor): add inline worksheet filters and ranges feat(sql-editor): add compact worksheet filters and ranges Oct 2, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
products/data_warehouse/frontend/bi/BIFilterControl.tsx (1)

68-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove duplicated between tooltip formatting.

The tooltip builds its own range string from raw filter.value and filter.valueTo. getBIFilterSummary already formats between ranges. The tooltip shows raw ISO strings for date-time fields, while the button shows formatted dates.

Use the full-precision raw bounds only if that is intended. Otherwise use summary for all operators.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: d4136880-5a1d-43eb-8973-7f72753838c9

📥 Commits

Reviewing files that changed from the base of the PR and between 85d889a and 2753278.

📒 Files selected for processing (11)
  • docs/published/handbook/engineering/data-warehouse.md
  • frontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFilterEditor.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFilterPill.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIFiltersCard.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIMarksCard.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIShelfCard.tsx
  • frontend/src/scenes/data-warehouse/editor/bi/components/BIShowMe.tsx
  • products/data_warehouse/frontend/bi/BIFilterControl.tsx

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

@posthog

posthog Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

✅ Visual changes approved by @mariusandra — baseline updated in 2ba4199.

View this run in PostHog

4 changed, 4 new.

Install the Visual Review Chrome extension to see visual review results at the top of your pull requests.

@trunk-io

trunk-io Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

🚫 This stack was removed from the merge queue because it was canceled by Marius Andra (a GitHub user). See more details here.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@trunk-io

trunk-io Bot commented Oct 3, 2026

Copy link
Copy Markdown

Stacked PR 111172 was cancelled: a user cancelled it.

@trunk-io

trunk-io Bot commented Oct 3, 2026

Copy link
Copy Markdown

Stacked PR 111178 was cancelled: a user cancelled it.

Base automatically changed from feat/bi-calculated-measures to master October 3, 2026 21:06
@mariusandra
mariusandra force-pushed the feat/bi-quick-filters branch from d1cf9d1 to c32f71c Compare October 4, 2026 00:29
@hosthog

hosthog Bot commented Oct 4, 2026

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

Latest build (c32f71c): https://364abb5aedfa497cb9a5c83ef2ec0c41.hosthog.dev

Employee-gated; every push gets a fresh URL whose content never changes. All previews stop serving when the PR closes.

This branch was successfully deployed

1 active deployment
preview-pr-111027 — c32f71c9 Deployed Oct 4, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant